-
Notifications
You must be signed in to change notification settings - Fork 2
Conversation
The weight of m4.large is higher than m3.large.
infrastructure/cloudformation.yml
Outdated
Type: AWS::EC2::SpotFleet | ||
Properties: | ||
SpotFleetRequestConfigData: | ||
IamFleetRole: !GetAtt [IAMFleetRole, Arn] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
=> IAMFleetRole.Arn
infrastructure/cloudformation.yml
Outdated
Properties: | ||
SpotFleetRequestConfigData: | ||
IamFleetRole: !GetAtt [IAMFleetRole, Arn] | ||
SpotPrice: '0.139' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
このあたりはパラメータ設定出来るようにする!
Type: Number | ||
TargetCapacity: | ||
Default: 2 | ||
Type: Number |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pataiji
来週レビューお願いします 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
差分、見やすい様に lambda-endpointをmerge先に向けています。
コードに問題無ければ masterにmergeします。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pataiji
指摘して貰った点修正したので、今日、暇な時にレビューお願いします!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2点コメントしました
infrastructure/cloudformation.yml
Outdated
Fn::Base64: !Sub | | ||
#!/bin/bash | ||
echo ECS_CLUSTER=${ECSCluster} >> /etc/ecs/ecs.config | ||
NetworkInterfaces: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
このプロパティはいらないような?
subnetでpublic ipを振るように設定しているので特に指定しなければ勝手にpub ip付けてくれると思います。
MapPublicIpOnLaunch: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
このプロパティは、NetworkInterfacesのDeleteOnTerminationをtrueにするために書きました。
( それ以外の属性は、require trueのもののみ書いてる状態です )
明示的に指定しない場合、Subnet設定だけで Network Interfaceが消えるという資料を見つけられなかったからです。とはいえ、この設定だといるかいらないか判断かつかないので、一回消してみて、Network Interfaceが消えなかったら、再度対応します!
infrastructure/cloudformation.yml
Outdated
LaunchSpecifications: | ||
# NOTE: | ||
# If you want to use other type of ec2 instances, | ||
# fix these parameters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parametersでinstanceType指定できるようにした方が良い気がします。(デフォルト設定はいれておいて)
weightを偏らせているのでMainInstanceTypeとSubInstanceTypeとか?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
たしかに!
parameterに入れといた方が、typeやidを変えたいときに、コード変えなくても大丈夫ですしね。
対応しておきます
infrastructure/cloudformation.yml
Outdated
SubContainerInstanceType: | ||
Default: 'm3.large' | ||
Type: String | ||
SubContainerInstanceImageId: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instance毎にimageを変えることあるかな?という気がしたのでこれに関しては共通のparametersで良いかなと思ったんですがどうでしょう?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
あ、たしかに。
別々にするメリット無いですね!
To change instance type and Id without modifying cloudformation.yml.
08bac3d
to
7656843
Compare
LGTM!!:+1: |
レビューありがとうございました! |
issue
#68
概要
最初は何も考えなくて済むように、とりあえず InstanceTypeと、ImageIdは決め打ち。
m4.large のほうが、weightが高くなるようにしている。
このあと IAMFleetRoleの設定していきます。
ami id の設定
http://docs.aws.amazon.com/ja_jp/AmazonECS/latest/developerguide/launch_container_instance.html
ecs用cloudformationの設定
http://docs.aws.amazon.com/ja_jp/AWSCloudFormation/latest/UserGuide/quickref-ecs.html